Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

Deny using relay chain for reserve transfers#1169

Merged
gilescope merged 16 commits intomasterfrom
giles-barrier
Apr 20, 2022
Merged

Deny using relay chain for reserve transfers#1169
gilescope merged 16 commits intomasterfrom
giles-barrier

Conversation

@gilescope
Copy link
Copy Markdown
Contributor

@gilescope gilescope commented Apr 12, 2022 •
edited
Loading

Fixes #5233

I will move DenyThenTry to polkadot xcm and then reference it from there in a later release, but we'd like to ship this for v900.

@paritytech-ci paritytech-ci requested review from a team April 12, 2022 17:37
KiChjang
KiChjang reviewed Apr 12, 2022
View reviewed changes
KiChjang
KiChjang reviewed Apr 12, 2022
View reviewed changes
KiChjang
KiChjang reviewed Apr 12, 2022
View reviewed changes
KiChjang
KiChjang reviewed Apr 12, 2022
View reviewed changes
KiChjang
KiChjang reviewed Apr 12, 2022
View reviewed changes
KiChjang
KiChjang reviewed Apr 12, 2022
View reviewed changes
KiChjang
KiChjang reviewed Apr 12, 2022
View reviewed changes
KiChjang
KiChjang reviewed Apr 12, 2022
View reviewed changes
@nuke-web3
Copy link
Copy Markdown
Contributor

nuke-web3 commented Apr 12, 2022

DumbQ: Any action needed for the template in this PR?

@NachoPal
Copy link
Copy Markdown
Contributor

NachoPal commented Apr 13, 2022

DumbQ: Any action needed for the template in this PR?

Considering that the template will be used mostly by future Parachains, I would say yes, it is a good idea to reject by default DepositReserveAsset and TransferReserveAsset if the destination is the MultiLocation { parents: 1, interior: Here }

The proper way to reserve transfer funds back to the reserve is by making use of InitiateReserveWithdraw

Unless somebody disagrees, can you @gilescope add also the change to the parachain-template runtime?

@NachoPal
Copy link
Copy Markdown
Contributor

NachoPal commented Apr 15, 2022

Has it been tested?

@gilescope
Copy link
Copy Markdown
Contributor Author

gilescope commented Apr 18, 2022

RE testing: I've tested that attempting to do a limited reserve asset transfer on statemine/statemint leads to a failure of Barrier. I've not run up a local westmint which does not filter Execute and tried executing individual instructions but that is something we could do.

@hbulgarini hbulgarini added B7-runtimenoteworthy T6-XCM This PR/Issue is related to XCM. labels Apr 19, 2022
@hbulgarini hbulgarini requested a review from KiChjang April 19, 2022 15:48
@gilescope
Copy link
Copy Markdown
Contributor Author

gilescope commented Apr 19, 2022

So just to confirm, I've checked that we can still do teleports back and forth between the relay and statemine, and that we can do a reserve asset transfer on statemine to the community parachain wtihout it getting blocked by the barrier.

KiChjang
KiChjang reviewed Apr 19, 2022
View reviewed changes
KiChjang
KiChjang reviewed Apr 19, 2022
View reviewed changes
hbulgarini
hbulgarini approved these changes Apr 20, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@hbulgarini hbulgarini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved.

@gilescope gilescope merged commit 563ce7c into master Apr 20, 2022
@gilescope gilescope deleted the giles-barrier branch April 20, 2022 09:14
@gilescope gilescope mentioned this pull request Apr 20, 2022
Merged
@github-actions github-actions bot mentioned this pull request Apr 20, 2022
Closed
@jasl jasl mentioned this pull request Apr 21, 2022
Merged
2 tasks
@gilescope gilescope mentioned this pull request Apr 22, 2022
Closed
jiguantong added a commit to darwinia-network/darwinia-parachain that referenced this pull request Jun 1, 2022
@jiguantong
Companion for paritytech/cumulus#1169
3341d8f
@jiguantong jiguantong mentioned this pull request Jun 8, 2022
Merged
2 tasks
@Dengjianping Dengjianping mentioned this pull request Jul 7, 2022
Merged
10 tasks
@github-actions github-actions bot mentioned this pull request Jul 18, 2022
Open
@joepetrowski joepetrowski mentioned this pull request Aug 10, 2022
Merged
5 tasks
aurexav
aurexav reviewed Nov 15, 2022
View reviewed changes
parachain-template/runtime/src/xcm_config.rs
}
}

// See issue #5233
Copy link
Copy Markdown
Contributor

@aurexav aurexav Nov 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be paritytech/polkadot#5233

Copy link
Copy Markdown
Contributor

@joepetrowski joepetrowski Nov 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, meant to make a follow up PR to docs in parachain-template. Will fix this too.

@aurexav aurexav mentioned this pull request Nov 23, 2022
Merged
@gilescope gilescope mentioned this pull request Apr 19, 2022
Closed
@bkontur bkontur mentioned this pull request Jan 15, 2025
Merged
2 tasks
@Agusrodri Agusrodri mentioned this pull request Jun 27, 2025
Merged
8 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@joepetrowski joepetrowski Awaiting requested review from joepetrowski

5 more reviewers

@KiChjang KiChjang KiChjang left review comments

@bkchr bkchr bkchr left review comments

@aurexav aurexav aurexav left review comments

@NachoPal NachoPal NachoPal approved these changes

@hbulgarini hbulgarini hbulgarini approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

T6-XCM This PR/Issue is related to XCM. T7-system_parachains This PR/Issue is related to System Parachains.

Projects

None yet

Milestone

statemint-v9.0.0

Development

Successfully merging this pull request may close these issues.

Not possible to reserve_transfer_assets back from Statemine to Kusama

8 participants

@gilescope @nuke-web3 @NachoPal @KiChjang @bkchr @hbulgarini @aurexav @joepetrowski